-
Notifications
You must be signed in to change notification settings - Fork 987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #36471 - Add eslint rule to alert missing ouia-ids #9765
Conversation
Issues: #36471 |
Issues: #36471 |
f2a839a
to
3b80c97
Compare
@@ -30,6 +30,7 @@ | |||
"@babel/core": "^7.7.0", | |||
"@theforeman/builder": "^12.0.1", | |||
"@theforeman/eslint-plugin-foreman": "^12.0.1", | |||
"@theforeman/eslint-plugin-rules": "^12.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add that to package-exclude.json
please :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to submit #9779 for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhmm, what happened here? It seems to be added here https://github.com/theforeman/foreman/pull/9765/files#diff-97f0a353de600204e809ec124bbc205b69ef828bbad153523a81472cea107aa5R6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I think I know what happened. It was removed in the rebase and yesterday I added it because of theforeman/foreman_remote_execution@ab46e19.
I still wonder if the -rules
package couldn't have been part of theforeman/eslint-plugin-foreman
, saving the whole dance.
3b80c97
to
56a7fe8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the packaging part is sane, so ACK on that.
I have no idea about the rest.
@lfu Hi, iirc you were involved in the foreman-js counterpart of this. Could you please take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
Thank you @MariaAga ! |
And fixing some missing ouia-ids.
Changed
lint:spelling
tolint:custom
as the command just runs all the custom rules defined in.eslintrc